Removes MASP record from storage #2363
Conversation
grarco
force-pushed
the
grarco/masp-storage-optimization
branch
from
73083fc
to
c7637ca
Compare
grarco
force-pushed
the
grarco/masp-storage-optimization
branch
from
01fb2b4
to
e1e050f
Compare
grarco
force-pushed
the
grarco/masp-storage-optimization
branch
from
41d0e30
to
19a532e
Compare
grarco
force-pushed
the
grarco/masp-storage-optimization
branch
from
19a532e
to
9408f4b
Compare
apps/src/lib/bench_utils.rs
Outdated
| tendermint_rpc::Error, | ||
| > | ||
| where | ||
| H: TryInto<namada::tendermint::block::Height> + Send, |
There was a problem hiding this comment.
why are the different trait bound on H as opposed to how it's done in the block method? In fact, the logic for checking the latest block heeight seems like it should be the same but it's implemented two different ways
There was a problem hiding this comment.
I changed the one in block_results because the Height type in the tendermint crate doesn't implement From<u64> but only TryFrom<u64>. I forgot to do the same modification for the block endpoint but if you agree I'd like to do that, otherwise we need to cast the block height to a u32 which can be converted without failing (but could always fail in the cast itself)
There was a problem hiding this comment.
Actually we have that trait bound in more endpoints so I might just revert my change
| .block | ||
| .data; | ||
|
|
||
| let tx = Tx::try_from(block[indexed_tx.index.0 as usize].as_ref()) |
There was a problem hiding this comment.
It seems like it might be possible that this and the code from fetch_shielded_transfers can be re-used here. I'm not quite sure as there is no wrapper check here, but it would be awesome if this stair-step code could be factored out into a separate function and re-used.
* origin/grarco/masp-storage-optimization: [feat] Refactoring extraction of shielded parts of txs to be more dry Shared function for shielded actions retrieval Reverts trait bound on `Height` for `block_results` endpoint Uses `masp_pin_tx_key` helper where possible Changelog #2363 `compute_pinned_balance` supports pinned masp over ibc transactions Improves masp over ibc tx fetching in client Refactors wrapper tx args for execution Cleans up sdk masp file Moves block query logic of integration tests in `block` Refactors and fixes benchmarks Refactors tx caching in `ShieldedContext` Fixes integration tests Updates masp queries to avoid the need for an indexer Fixes integration tests Fetches masp txs from blocks Adds a new tx event attribute to index masp txs Updates keys check in masp vp Updates masp storage keys helper functions Removes masp tx writing of lookup data to storage
Describe your changes
Addresses point 2 of #2343.
Removes the masp record from storage except for the pin key. Queries are now done via the cometBFT rpc endpoints
\blockandblock_results. Also updates key validation in the masp vp.Adds a new
is_valid_masp_txattribute to appropriate tx events infinalize_blockto allow for a fast lookup on the client side.Updates the mock implementations of
Clientfor integrations tests and benchmarks to support the new queries.Indicate on which release or other PRs this topic is based on
#2345 - diff for review: https://github.com/anoma/namada/pull/2363/files/ddd6368725255f0c6bd468d7e2d849f8b0a407a6..454e138f012e6c522277d3ac69b50f2a5763c982
Checklist before merging to
draft